Skip to content

Conversation

@ronakbanka
Copy link

Aim of this PR is to change the property name from use_ssl to require_ssl for cfdot , to maintain uniformity among job properties.

As all other diego jobs are using require_ssl property to enable or disable communication with bbs.

auctioneer
nsync
rep
ssh-proxy

@cfdreddbot
Copy link

Hey ronakbanka!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/141679493

The labels on this github issue will be updated when the story is started.

@emalm
Copy link
Contributor

emalm commented Mar 16, 2017

Hi, @ronakbanka,

Thanks for pointing out the inconsistency. We can't just rename that property right away as you have suggested, as the removal of the current property would be a breaking change to the BOSH-property interface of the release and so must wait until we're ready to release the next major version of diego-release. Now that BOSH is recommending that properties be specified on a per-job basis within each instance group, instead of as global properties, we're planning to do a more drastic replacement of these properties to drop the diego.X namespaces and to consolidate some of the TLS credentials that are cropping up everywhere. This one would likely become bbs.require_tls under that new property scheme.

Best,
Eric, CF Diego PM

@ronakbanka
Copy link
Author

@ematpl got the point Eric. Simply renaming right away can break things. Until then we can live with that property 👍

@emalm
Copy link
Contributor

emalm commented Mar 16, 2017

Great, I'll close this PR out, and the Diego team will be tracking the TLS reconsolidation work in https://www.pivotaltracker.com/story/show/141835465. It also occurs to me that we're likely to require TLS for all API communication in Diego v2 anyway, so this property itself may then be obsolete.

Thanks again,
Eric

@emalm emalm closed this Mar 16, 2017
bdshroyer pushed a commit that referenced this pull request Mar 27, 2017
[finishes #142522481]

Submodule src/github.com/onsi/ginkgo c3a655f..67b9df7:
  > Remove the spec_iterator.test binary (#336)
  > Shared queue implementation for parallel tests
  > Revert "Don't colorize output by default if not writing to a TTY (#328)" (#331)
  > Don't colorize output by default if not writing to a TTY (#328)
  > Use SVG badge for build status (#330)
  > Add the ability to use ./... to recursively test directories (#319)
  > Include captured output from failed tests into JUnit (#318)
  > Aggregate flaked specs (#316)
  > Add colours for Windows in suite-runner & watch (#312)
  > Fix tests for single node machine (#311)
  > Add ability to specify a custom bootstrap file (#302)
  > Revert "remove -i in invocations of go test.  fixes #305"
  > Update .travis.yml
  > fix imports for generate command (#279)
  > Merge branch 'koron-windows-colorise'
  > remove -i in invocations of go test.  fixes #305
  > remove unnecessary variable
  > backfill GinkgoRandomSeed test
  > Expose the random seed via GinkgoRandomSeed() (#293)
  > Include flake count in test summary (#291)
  > #287 Ensure Logf/Skipf insert newline characters (#288)
  > Add package path prefix to compilation output path only if missing (#284)
  > Redo flags again, add a bunch of pass-throughs. (#282)
  > Spelling fix (#283)
  > Covermode flag (and reworked pass-through flags passing) (#281)
  > Make JUnit reporter include failure location in message. (#262)
  > remove 1.4 from travis.yml
  > Add gcflags option (#276)
  > Revert "Use the go1.5 build tag to handle vendor exceptions" (#274)
  > Merge pull request #272 from fsouza/fix-vendor
  > Add flaky test mitigation (#261)
  > Allow units and precision in benchmark (#266)
  > Add Solaris support (#264)
  > Merge pull request #259 from kwadrat/master
  > Merge branch 'apvail-spell-fix'
  > Fix go16 vendor
  > Merge pull request #250 from james-lawrence/master
  > Merge pull request #228 from jayunit100/RegexFileNameFiltering
  > Fix test flakiness
Submodule src/github.com/onsi/gomega c463cd2..334b8f4:
  > Merge pull request #206 from xoebus/patch-1
  > Merge pull request #205 from onsi/revert-201-json_formatting
  > Merge pull request #201 from madamkiwi/json_formatting
  > Merge pull request #199 from kevgo/patch-1
@ronakbanka ronakbanka deleted the sync_cfdot_require_ssl_property branch July 24, 2017 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants